fix(action menu): keyboard accessibility omnibus #5031

nikkimk · 2025-01-16T21:42:58Z

Action menu items are not reading for screen readers.

Description

Action menu should be using a roving tabindex not aria-activedescendant because of cross-root ARIA limitations as well as lack of iOS support.

The sp-menu that action menu uses was refactored to use a roving tabindex, and the numpad keys fix that was made in action menu are now applied to the focus group controller which the roving tabindex controller uses.

Related issue(s)

[Bug]: sp-menu does not honor Numpad keys for Arrow ⬆️ ⬇️ and Enter #3751
[Bug]: When navigating with Voice Over in iOS moving forward through Menu Items in a Picker forces focus to the first Menu Item #3774
[Bug]: Arrow keys don't work on Picker and Action-Menu if its opened by a mouse click. #4532
[Bug][A11y]: Action-Menu menu items doesnt read in screen-readers #4556
[Bug]: Keyboard navigation of menu items doesn't start with the first item #4557
[Bug]: sp-menu-group causes nested menus when no selection #4614
[Bug][A11y]: Menu item with href fails to activate using a screen reader #4997

Motivation and context

VoiceOver could not read the menu items when navigated via keyboard because of the cross-root aria issues above. Using the same roving tabindex controller that other components in our repo use, allows us to ensure roving tabindex and keyboard navigation is accessible and consistent across all components.

How has this been tested?

By default a Menu should follow the Navigation Menu Example from the WAI ARIA APG.
Menus with selects and menu groups should function like the menu groups in the Editor Menubar Example from the WAI ARIA APG
Please note that there will be VRT differences where open menus that didn't used to have focus now do. Menus that are opened via keyboard are supposed to set focus on a menu item.

Does screenreader read menuitems? (resolves #4556 and without regression on #3751)

Go to https://nikkimk-fix-menu-a11y--spectrum-web-components.netlify.app/components/action-menu/#sizes
Open Voice Over
Tab to the More Actions... menu button.
Press Down Arrow repeatedly to scroll through the menu items
Press Up Arrow repeatedly to scroll through the menu items
Press Numpad Down Arrow repeatedly to and scroll through the menu items
Press Numpad Up Arrow repeatedly to and scroll through the menu items
Screenreader should read menu items

Does picker menuitems selection work in VoiceOver on iOS (resolves #3774)

In iOS with Voice Over on go to https://nikkimk-fix-menu-a11y--spectrumwc.netlify.app/storybook/?path=/story/picker--default
Focus one of the Picker elements
Open the Picker's Menu
Attempt to swipe to the second or later item in the Menu
See the focus moves to the correct item

Can you use a screenreader to click a menuitem? (resolves #4997)

Go to https://nikkimk-fix-menu-a11y--spectrum-web-components.netlify.app/components/action-menu/#sizes
Open Voice Over
Tab to the menuitem.
Tab focus to the button.
Press CRTL+Option+Space to click the link.
Link should be activated.

Does keyboard navigation of menuitems work as it should? (resolves #4532 and #4557)

Go to this WAI ARIA APG Menu Button example.
Click on the Actions menu button.
Notice that the menu opens and focus is on the first item.
Press Down Arrow.
Notice that the focus is on the second item.
Now go to https://nikkimk-fix-menu-a11y--spectrum-web-components.netlify.app/storybook/iframe.html?args=&id=action-menu--default&viewMode=story
Click on the More Actions... menu button.
Notice that the menu opens and focus is on the first item.
Press Down Arrow.
Notice that the focus is on the second item.

Do menu groups avoid nested menus? (resolves #4614)

Go to https://nikkimk-fix-menu-a11y--spectrum-web-components.netlify.app/iframe.html?args=&id=menu-group--complex-slotted&viewMode=story
Inspect the accessibility tree
Expand the menu in the accessibility tree.
Expand each group within the menu.
See there are nested menus

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
[] Breaking change (fix or feature that would cause existing functionality to change) _This change will affect combobox, but combobox was already inaccessible. See RFC for Accessible Menu navigation for more information. _
Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

…endant

changeset-bot · 2025-01-16T21:43:02Z

🦋 Changeset detected

Latest commit: b7398ad

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2025-01-16T21:47:11Z

Tachometer results

Currently, no packages are changed by this PR...

github-actions · 2025-01-16T21:49:01Z

Lighthouse scores

Category	Latest (report)	Main (report)	Branch (report)
Performance	0.99	0.99	0.99
Accessibility	1	1	1
Best Practices	1	1	1
SEO	1	0.92	0.92
PWA	1	1	1

What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category	Latest	Main	Branch
Total	244.364 kB	230.654 kB 🏆	230.867 kB
Scripts	60.705 kB	54.239 kB 🏆	54.381 kB
Stylesheet	47.543 kB	41.816 kB 🏆	41.991 kB
Document	6.223 kB	5.485 kB 🏆	5.491 kB
Font	126.92 kB	126.604 kB 🏆	126.654 kB

Request Count

Category	Latest	Main	Branch
Total	52	52	52
Scripts	41	41	41
Stylesheet	5	5	5
Document	1	1	1
Font	2	2	2

coveralls · 2025-01-16T21:52:04Z

Pull Request Test Coverage Report for Build 13553164764

Details

627 of 678 (92.48%) changed or added relevant lines in 11 files are covered.
27 unchanged lines in 8 files lost coverage.
Overall coverage decreased (-0.2%) to 97.966%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/picker/src/MobileController.ts	3	5	60.0%
packages/menu/src/Menu.ts	212	215	98.6%
tools/reactive-controllers/src/FocusGroup.ts	52	55	94.55%
packages/combobox/src/Combobox.ts	9	13	69.23%
packages/picker/src/Picker.ts	162	170	95.29%
packages/menu/src/MenuItem.ts	138	153	90.2%
packages/action-menu/src/ActionMenu.ts	12	28	42.86%

Files with Coverage Reduction	New Missed Lines	%
packages/menu/src/MenuItem.ts	1	95.01%
packages/picker/src/DesktopController.ts	2	90.83%
packages/picker/src/InteractionController.ts	2	94.35%
packages/tabs/src/Tabs.ts	2	98.34%
tools/shared/src/focusable.ts	2	99.32%
packages/menu/src/Menu.ts	3	95.54%
tools/reactive-controllers/src/FocusGroup.ts	3	97.47%
packages/picker/src/Picker.ts	12	96.32%

Totals
Change from base Build 13548242533:	-0.2%
Covered Lines:	33662
Relevant Lines:	34164

💛 - Coveralls

…child WAI ARIA APG

…d nav

…nikkimk/fix-menu-a11y

nikkimk · 2025-01-30T15:45:48Z

packages/action-menu/src/ActionMenu.ts

@@ -135,4 +135,32 @@ export class ActionMenu extends ObserveSlotPresence(
        }
        super.update(changedProperties);
    }
+


Action Menu was throwing the no label warning for a test that used the label-only slot, so I decided to override Picker's accessible label check and warning with a warning specific to action menu.

…nikkimk/fix-menu-a11y

majornista · 2025-02-26T18:07:14Z

In the Picker > Matching value and Picker > Matching itemText examples, when the Picker opens with a value selected for the first time, focus is going to the first menuitem rather than the selected value. On subsequent opening, the selected value will receive focus. It seems like the selected value for the menu does not initialize until it is rendered.

This should be resolved. Take a look.

With MenuItem > Submenu, Space key scrolls (per @jnurthen comment above), and with Enter key, the submenu opens, but focus does not move to the submenu. Subsequent keyboard navigation using arrow keys, navigates the current menu rather than the opened submenu. Similarly, activating a submenu parent item using the VoiceOver cursor fails to focus the submenu.

This should be resolved. Take a look.

The second part of this concern, submenu parent item using the VoiceOver cursor fails to focus the submenu, is still a problem.

This can be tested by navigating to a submenu parent item with VoiceOver using the VoiceOver cursor, control+option+down arrow, then using control+option+Space to trigger a click event on the menu item using the VoiceOver cursor. The click event triggers, and the submenu will open, but the focus does not shift to the submenu, and you cannot access the submenu by navigating with the VoiceOver cursor.

This issue seems resolved.

I also notice that each submenu parent item has aria-expanded="true" when the submenu is not open. aria-expanded should update to reflect the current state of the submenu.

This issue seems resolved.

Using the VoiceOver on iOS to test the ActionMenu > Submenu story, after opening the menu in a tray, double tapping "Select some items" to activate the submenu closes the tray rather than opening the submenu. The submenu story is similarly broken without VoiceOver on.

This relates and is dependent on #5076 which is an existing issue on our main branch, so I filed #5110

This issue is behaving better, but still a bit odd. When the submenu opens after clicking "Select some items" from the tray menu, VoiceOver on iOS seems to focus the items in the submenu, but the submenu appears to be hidden offscreen to the right, as if the menu were rendered using a forced popover. Within a tray, submenus should probably behave as they currently behave in React-Spectrum: https://react-spectrum.adobe.com/react-spectrum/Menu.html#dynamic, where the submenu slides in to replace the parent menu within the tray, and includes a button back to the parent menuitem at the top, in order to dismiss the submenu.

We could treat the submenu with in a tray design as a separate concern.

Using VoiceOver on iOS to test any of the examples that open the menu in a tray like ActionMenu >Default story, double tapping to open fails to focus an item in the menu, and double tapping to select an item or to dismiss the menu using the visually hidden dismiss button fails to restore focus to the button element that opened the menu.

This should be resolved. Take a look.

The menuitem is receiving focus as expected when the menu opens. However, when the menu closes after selecting an item or double tapping the visually hidden dismiss button, focus does not get restored to the action button element that opened the menu. Instead, VoiceOver seems to automatically focus a nearby element, which may be somewhere near the bottom of the page. In Storybook on an iphone, it seems to be focusing one of the tabs at the bottom of the Storybook UI, "Sidebar," "Canvas," "Settings," or "Addons." The result is an unexpected change in context which will be problematic for screen reader users when the page content is longer.

This seems to be resolved.

majornista

See comment: #5031 (comment)

Rajdeep approved after Tarun's day ended, unblocking reviews

github-actions · 2025-02-26T19:16:24Z

Branch preview

Review the following VRT differences

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

* fix(menu): added rovingTabindexController and removed aria-activedescendant * fix(menu): fixes firefox keyboard navigation and call stack issues * test(menu): updates menu test based on changes * test(menu): adds click test for accessibility * test(menu): menu itself should delegate focus to active item * fix(menu): fixed menu group navigation according WAI ARIA APG * fix(reactive-controllers): complex element delegates focus to active child WAI ARIA APG * fix(menu): components should delegate focus * fix(menu): update keyboard nav to match WAI ARIA APG * fix(menu): updated docs to reflect preferred accessible method * fix(menu): selection and keyboard navigation according to APG * test(menu): updated to recommended a11y behavior from APG * docs(menu): reverted docs but fixed typos * fix(menu): removed unused map * test(menu): updated test to match changed group id * test(menu): keyboard selection is fixed * test(menu): updated to reflect WAI ARIA APG * fix(picker): picker should delegate focus * fix(action-menu): fixed a11y error message * fix(picker): according to APG ArrowDown should focus on first item * test(action-menu): updated tests for focus delegation and APG keyboard nav * fix(picker): ensure overlay is open before setting focus * fix(menu): removed extra focus * chore: added changeset * fix(picker): prevents the picker button from getting focus when menu is first updated * fix(menu): fixes preventDefault behavior when Space is pressed * fix(menu): accounted for action menus with no selection * fix(picker):sets focus on first selected item * fix(picker): focuses on 1st selected item when opened via kbd * test(picker): updated tests based on APG kbd nav * fix(menu): when I menuitem with a submenu closes * fix(picker): leverages RTI for ArrowLeft/ArrowRight * fix(picker): leverages RTI for ArrowLeft/ArrowRight * test(picker): adjusted tests based on a11y requirements * fix(picker): fixed accessibility warning * fix(picker): resolves opening and closing tests * fix(menu): resolves test by setting focus after scrolling * fix(picker): resolves mobile test * test(menu): updated tests for focus delegation * fix(picker): ensures that slotted labels don't get a11y warning * fix(picker): resolves menu close issue * fix(menu): fixed prev and next item * fix(picker): fixed left-right arrow keys * test(tray): debugging VO/iOS tray * fix(menu): resolves submenu tests * fix(menu): added some temporary debugging logic and comments * test(action-menu): removed test that no longer applies to a11y * fix(action-menu): fixed menu closing issue, added docs, and removed temporary debugging * test(action-menu): fixed test based on focus behavior when an item is selected * merge: merge from main * merge: merge from main * test(combobox): removing aria-activedescendant references until a combobox solution is implemented * test(combobox): removing aria-activedescendant references until a combobox solution is implemented * test(combobox): added skipped tests back * fix(combobox): resolves timing issues with updated menu * test(combobox): updated test docs * chore(combobox): removed debugging * docs(combobox): updated webkit escape hatches * test(combobox): removed isWebkit since escape hatch is not needed * test(tabs): removed test that does not follow WAI ARIA APG * fix(controllers): delegates focus setting until all other components using controllers are reviewed for a11y, * fix(controllers): fix(controllers): delegates focus setting until all other components using controllers are reviewed for a11y, * test(action-menu): fixed typo * test(action-menu): fixed typo * chore: fix menu tests with comments for skips * chore: cleanup * chore: rebuilt stylesheet * chore: dialog stylesheet * chore: dialog stylesheet * chore: modified pre-commit hook temporarily and fix spectrum-dialog.css * chore: revert hook * fix(menu): ensures menu used focus delegation * fix(action-group): should delegate focus * fix(action-group): prevents submenu events from reaching action group * fix(action-group): prevents submenu events from bubbling * fix(action-group): preventing submenu actions from being handled by action group * fix(action-menu): prevents submenu key events from being handle by action group * chore: added tons of comments to picker test * chore: added comments to action menu test * chore: added comments to action group test * fix(picker): fixed focus delegation * fix(menu): allows Escape to bubble to action menu and picker * fix(picker): fixes escape key and disabled closing * fix(picker): fixes escape key and disabled closing * test(menu): added more info to submenu test * fix(reactive-controllers): fixed failing reactive-controller tests * fix(picker): resolves focus styles in VRT * docs(tray): added a story to test buttons in tray on mobile * docs(tray): removed unnecessary story * docs(action-menu): added an example for #4623 * fix(menu): fixes for menu focus issues on chromium as well as updating menuitems * fix(menu): resolved all but a WebKit timeout on removal * chore: reverting last commit * fix(menu): fixes failing tests related to mutations * chore: styles from update from main * test(menu): removed skip from removal test * fix(combobox): fixes combobox focus issue * chore: updated CSS * chore: updated golden hash * fix(menu): menu item removal and focus * fix(menu): fixed aria-expanded in submenus * fix(menu): ensures sunmenu recieves focus when opened * fix(picker): ensures focus is not lost when overlay closes * fix(menu): resolves menu removal issue * chore: updated hash for new stories and a11y behaviors * fix(action-menu): resolves action-menu timeout * test(picker): fixed test based on a11y focus * test(picker): fixed timeout on picker test * test(action-menu): replaced removed test * chore(picker): fix missing import from merge * chore(picker): fixed merging issue * chore: reverting hash to main for comparison * fix(picker): focus should be managed by user not initial state * fix(picker): picker overlay should only focus with a user action * fix(picker): picker overlay should only focus with a user action * test(picker): fix timeout * test(picker): visual diffs because focus should only be set by user action * fix(menu): made submenu items focusable after open * fix(menu): fixes menuitem focus * test(menu): temporarily skipping Chrome * chore: reverting to main has for comparison * fix(menu): fixed menu test * fix(menu): fixed menu test * chore: updated golden images hash * fix(menu): fixes VoiceOver submenu focus Issue * fix(picker): retains focus after user closes menu * test(picker): uncommented tests * test(picker): fixed focus throw test * test(picker): removed `only` * test(picker): added test for retaining focus * chore: resolved hash conflict * chore: reverting hash to resolve * chore: updating hash * chore: reverting hash * chore: reverting hash * chore: reverting hash * chore: updating hash again * test(action-menu): added tests for returning focus * test(action-menu): added a11y test * test(action-menu): added a11y test * tes(action-menu): removed an `only` * test(action-menu): fixed linting issue --------- Co-authored-by: Casey Eickhoff <[email protected]>

nikkimk added 4 commits January 16, 2025 16:06

fix(menu): added rovingTabindexController and removed aria-activedesc…

91b72f7

…endant

fix(menu): fixes firefox keyboard navigation and call stack issues

adc0d26

test(menu): updates menu test based on changes

97d88c3

test(menu): adds click test for accessibility

d10ed4c

nikkimk linked an issue Jan 16, 2025 that may be closed by this pull request

[Bug][A11y]: Action-Menu menu items doesnt read in screen-readers #4556

Closed

1 task

nikkimk added 20 commits January 17, 2025 09:14

test(menu): menu itself should delegate focus to active item

cb74cba

fix(menu): fixed menu group navigation according WAI ARIA APG

3876c36

fix(reactive-controllers): complex element delegates focus to active …

350e9ba

…child WAI ARIA APG

fix(menu): components should delegate focus

b7c750f

fix(menu): update keyboard nav to match WAI ARIA APG

68a298d

fix(menu): updated docs to reflect preferred accessible method

63e1dab

fix(menu): selection and keyboard navigation according to APG

b6ee246

test(menu): updated to recommended a11y behavior from APG

ef99ebb

docs(menu): reverted docs but fixed typos

eb7e539

fix(menu): removed unused map

36cc4a2

test(menu): updated test to match changed group id

9f5f16c

test(menu): keyboard selection is fixed

01be995

test(menu): updated to reflect WAI ARIA APG

c5a9514

fix(picker): picker should delegate focus

a949c61

fix(action-menu): fixed a11y error message

62147ea

fix(picker): according to APG ArrowDown should focus on first item

8211077

test(action-menu): updated tests for focus delegation and APG keyboar…

66eb2d8

…d nav

fix(picker): ensure overlay is open before setting focus

7f6b363

fix(menu): removed extra focus

d4f7f93

Merge branch 'main' of github.com:adobe/spectrum-web-components into …

291231f

…nikkimk/fix-menu-a11y

nikkimk commented Jan 30, 2025

View reviewed changes

nikkimk requested a review from rubencarvalho February 26, 2025 15:30

nikkimk added 13 commits February 26, 2025 10:37

test(picker): uncommented tests

41fdeb2

test(picker): fixed focus throw test

c399545

test(picker): removed only

bd377ce

test(picker): added test for retaining focus

c492e0c

chore: resolved hash conflict

c76d18d

chore: reverting hash to resolve

d620f22

chore: updating hash

090ac70

chore: reverting hash

49fb778

chore: reverting hash

944ca34

chore: reverting hash

6161219

Merge branch 'main' of github.com:adobe/spectrum-web-components into …

34923d0

…nikkimk/fix-menu-a11y

chore: updating hash again

7a414dd

test(action-menu): added tests for returning focus

a99d776

Rajdeepc approved these changes Feb 26, 2025

View reviewed changes

caseyisonit approved these changes Feb 26, 2025

View reviewed changes

majornista approved these changes Feb 26, 2025

View reviewed changes

adobe deleted a comment from github-actions bot Feb 26, 2025

caseyisonit force-pushed the nikkimk/fix-menu-a11y branch from 6251f5f to a99d776 Compare February 26, 2025 19:55

nikkimk added 4 commits February 26, 2025 15:31

test(action-menu): added a11y test

c57f60e

test(action-menu): added a11y test

4e005fd

tes(action-menu): removed an only

a968404

test(action-menu): fixed linting issue

b7398ad

nikkimk merged commit ea38ef0 into main Feb 26, 2025
24 of 27 checks passed

nikkimk deleted the nikkimk/fix-menu-a11y branch February 26, 2025 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(action menu): keyboard accessibility omnibus #5031

fix(action menu): keyboard accessibility omnibus #5031

Uh oh!

nikkimk commented Jan 16, 2025 •

edited

Loading

Uh oh!

changeset-bot bot commented Jan 16, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jan 16, 2025

Uh oh!

github-actions bot commented Jan 16, 2025 •

edited

Loading

Uh oh!

coveralls commented Jan 16, 2025 •

edited

Loading

Uh oh!

nikkimk Jan 30, 2025

Uh oh!

majornista commented Feb 26, 2025

Uh oh!

majornista left a comment

Uh oh!

github-actions bot commented Feb 26, 2025

Uh oh!

Uh oh!

Uh oh!

fix(action menu): keyboard accessibility omnibus #5031

fix(action menu): keyboard accessibility omnibus #5031

Uh oh!

Conversation

nikkimk commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issue(s)

Motivation and context

How has this been tested?

Does screenreader read menuitems? (resolves #4556 and without regression on #3751)

Does picker menuitems selection work in VoiceOver on iOS (resolves #3774)

Can you use a screenreader to click a menuitem? (resolves #4997)

Does keyboard navigation of menuitems work as it should? (resolves #4532 and #4557)

Do menu groups avoid nested menus? (resolves #4614)

Types of changes

Checklist

Best practices

Uh oh!

changeset-bot bot commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Jan 16, 2025

Tachometer results

Uh oh!

github-actions bot commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lighthouse scores

Transfer Size

Request Count

Uh oh!

coveralls commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 13553164764

Details

💛 - Coveralls

Uh oh!

nikkimk Jan 30, 2025

Choose a reason for hiding this comment

Uh oh!

majornista commented Feb 26, 2025

Uh oh!

majornista left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 26, 2025

Branch preview

Review the following VRT differences

Uh oh!

Uh oh!

Uh oh!

nikkimk commented Jan 16, 2025 •

edited

Loading

changeset-bot bot commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025 •

edited

Loading

coveralls commented Jan 16, 2025 •

edited

Loading